refactor: Separate adding/removing cycles from refunding/consuming cycles#9717
Merged
mraszyk merged 4 commits intodfinity:masterfrom Apr 7, 2026
Merged
refactor: Separate adding/removing cycles from refunding/consuming cycles#9717mraszyk merged 4 commits intodfinity:masterfrom
mraszyk merged 4 commits intodfinity:masterfrom
Conversation
Contributor
Contributor
|
Seeing this error: |
mraszyk
reviewed
Apr 2, 2026
mraszyk
reviewed
Apr 2, 2026
mraszyk
reviewed
Apr 2, 2026
mraszyk
reviewed
Apr 2, 2026
mraszyk
reviewed
Apr 2, 2026
mraszyk
approved these changes
Apr 7, 2026
Contributor
Contributor
Author
|
@mraszyk Seems something failed on the merge queue twice. Unclear to me if the timeout is a real problem or not. It is plausible my changes are bringing the time over as I'm adding a few extra |
Contributor
|
@dsarlis arm64-linux is often flaky so I just added this PR to the merge queue again... |
Contributor
Author
|
It's EDIT: the second time, first time |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR refactors how cycles changes to balance and metrics are happening via the
SystemState. Specifically, it more clearly separates adding/removing cycles to the balance from refunding/consuming cycles which on top of the balance updates the respective consumed metrics as well.The idea is to separate in two pairs of APIs that will be responsible for each type of update:
to handle balance only changes while
will handle metrics being updated as well.
This change has a two fold benefit.
On one hand, it allows us to perform direct updates to balance (e.g. when depositing cycles or attaching cycles in calls) without having to construct
CompoundCycles<NonConsumed>which was a somewhat hacky way to say "no need to update metrics". In fact, after this change,NonConsumedcan be completely removed fromCyclesUseCases. This allows for some simplification in call sites where they don't need to deal withCompoundCyclesor doing so incurs unnecessary extra work.On the other, it allows us to require a prepayment for refunding cycles which would be very helpful in a follow up where a metric which acts as a counter will be introduced and the amount consumed will need to be computed based on prepayment and refund. This way the call sites that don't need or should not deal with prepayments (i.e. ones that need only
add_cycles), they don't have to be exposed to any of this.The meat of the changes are in
SystemStatewhile there are some updates in production code and tests to match the refined API. In many cases, it ends up in a simpler version of the code as a bunch ofCompoundCycles::newcalls are eliminated or the cost schedule does not need to be piped through various calls.